-
Notifications
You must be signed in to change notification settings - Fork 18
Stop hiding cause of last exception #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2999255 to
9985364
Compare
9985364 to
56d9b6e
Compare
| def __str__(self) -> str: | ||
| return str(self.message) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since message is passed in super.__init__(message) and we removed the custom __str__ implementation below, this is already the default implementation.
In Python, __str__ isn't helpful. If you need to know the class name, then __repr__ is required, and our __repr__ implementation contains all information about the current exception, including errors.
>>> e = ValueError("abandon ship!")
>>> str(e)
'abandon ship!'
>>> repr(e)
"ValueError('abandon ship!')"| def __str__(self) -> str: | ||
| if self.errors: | ||
| return f"Connection error caused by: {self.errors[0].__class__.__name__}({self.errors[0]})" | ||
| return "Connection error" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This __str__ implementation tries to be helpful but hides the current exception to focus on errors, which are past attempts. This comment applies to the other subclasses as well.
| def exception_to_dict(exc: TransportError) -> dict: | ||
| return { | ||
| "type": exc.__class__.__name__, | ||
| "message": exc.message, | ||
| "errors": [exception_to_dict(e) for e in exc.errors], | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all the information exposed by __repr__, but in a more structured way which is nicer to test for.
Closes #223
While this does not change any types, it does change the values returned by
str(e)andrepr(e), so I would like to include this in 9.0 only, without backporting to 8.x.